Skip to content

Ensure we always startup with a rustls CryptoProvider (main) #601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

tnull
Copy link
Collaborator

@tnull tnull commented Aug 1, 2025

Closes #595. (for now)

The rustls library recently introduced this weird behavior where they expect users to, apart from configuring the respective feature, also explictly call CryptoProvider::install_default. Otherwise they'd simply panic at runtime whenever the first network call requiring TLS would be made. While we already made a change upstream at rust-electrum-client, we also make sure here that we definitely, always, absolutley are sure that we have a CryptoProvider set on startup.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Aug 1, 2025

🎉 This PR is now ready for review!
Please choose at least one reviewer by assigning them on the right bar.
If no reviewers are assigned within 10 minutes, I'll automatically assign one.
Once the first reviewer has submitted a review, a second will be assigned if required.

@tnull tnull requested a review from joostjager August 1, 2025 12:29
@tnull tnull force-pushed the 2025-08-fix-rustls-crypto-provider-main branch 2 times, most recently from 0e4507b to 2cdcea3 Compare August 1, 2025 13:16
@tnull
Copy link
Collaborator Author

tnull commented Aug 1, 2025

Putting in draft until we're clear what's going on (give we just saw a CI job where a single test case hit the introduced assert). What a mess.

@tnull tnull marked this pull request as draft August 1, 2025 13:50
@tnull tnull removed the request for review from joostjager August 1, 2025 13:50
@tnull tnull force-pushed the 2025-08-fix-rustls-crypto-provider-main branch from 2cdcea3 to ac70b2f Compare August 12, 2025 12:51
@tnull tnull requested a review from TheBlueMatt August 12, 2025 13:02
@tnull tnull marked this pull request as ready for review August 12, 2025 13:02
Copy link

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments as #600 (review) lets discuss there.

@tnull tnull force-pushed the 2025-08-fix-rustls-crypto-provider-main branch 2 times, most recently from 8ac392d to 4887c75 Compare August 14, 2025 08:14
The `rustls` library recently introduced this weird behavior where they
expect users to, apart from configuring the respective feature, also
explictly call `CryptoProvider::install_default`. Otherwise they'd
simply panic at runtime whenever the first network call requiring TLS
would be made. While we already made a change upstream at
`rust-electrum-client`, we also make sure here that we definitely,
always, absolutley are sure that we have a `CryptoProvider` set on
startup.
@tnull tnull force-pushed the 2025-08-fix-rustls-crypto-provider-main branch from 4887c75 to 1d06c7a Compare August 14, 2025 08:22
@tnull tnull merged commit 3de8b1d into lightningdevkit:main Aug 14, 2025
9 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix panic when accessing ssl:// Electrum server
3 participants